Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing unsubscribe in RealtimePresencePromise #608

Closed
wants to merge 1 commit into from

Conversation

kazk
Copy link
Contributor

@kazk kazk commented Sep 10, 2019

No description provided.

@SimonWoolf
Copy link
Member

Thanks @kazk 🙂 You're right that the typings are missing unsubscribe, though the signature in this pr isn't quite right — you can't pass an array of actions to unsubscribe and it doesn't return a promise (it's a synchronous action). I've fixed in master by moving the RealtimePresenceCallbacks.unsubscribe definition to RealtimePresenceBase: 1624f93

@SimonWoolf SimonWoolf closed this Sep 10, 2019
@kazk
Copy link
Contributor Author

kazk commented Sep 10, 2019

Thanks :)

it doesn't return a promise

Yeah, I missed that only subscribe has callbackWhenAttached?.

you can't pass an array of actions to unsubscribe

The documentation mentions it

image

https://www.ably.io/documentation/realtime/presence#unsubscribe

@kazk kazk deleted the fix-missing-presence-unsubscribe branch September 10, 2019 18:23
@SimonWoolf
Copy link
Member

@kazk re unsubscribe with an array: right you are, I'd forgotten I implemented that 🙂. Fixed in 45e8781

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants